Skip to content

Conversation

@MananTank
Copy link
Member

@MananTank MananTank commented Apr 17, 2024

PR-Codex overview

This PR introduces minor improvements to the thirdweb Pay UI.

Detailed summary

  • Updated defaultTokens icon property to be optional
  • Changed position value in Drawer component from "fixed" to "absolute"
  • Modified token parameter in isNativeToken function to be Partial
  • Added formatTokenBalance function for token balance formatting
  • Created openOnrampPopup function for opening onramp popups
  • Added PayTokenInfo and PayOnChainTransactionDetails types
  • Implemented invalidateWalletBalance function
  • Introduced LazyBuyScreen component for lazy loading
  • Updated settings in settings.json for TypeScript
  • Added types for BuyForTx and SelectedScreen
  • Created StepBar component for step tracking
  • Updated imports in getHistory and pendingSwapTx files
  • Added CADIcon component
  • Updated imports in ConnectEmbed and ChainName files
  • Updated exports in thirdweb.ts
  • Implemented invalidateWalletBalance function in ThirdwebProvider
  • Added freezeAmount and freezeChainAndToken props in BuyTokenInput component

The following files were skipped due to too many changes: packages/thirdweb/src/react/web/ui/components/SwitchNetwork.tsx, packages/thirdweb/src/react/web/ui/components/buttons.tsx, packages/thirdweb/src/react/web/ui/ConnectWallet/screens/Buy/tx-history/TxDetailsScreen.tsx, packages/thirdweb/src/react/web/ui/components/token/TokenSymbol.tsx, packages/thirdweb/src/react/web/ui/ConnectWallet/screens/Buy/fiat/CurrencySelection.tsx, packages/thirdweb/src/react/web/ui/ConnectWallet/icons/currencies/EURIcon.tsx, packages/thirdweb/src/react/core/hooks/contract/useSendTransaction.ts, packages/thirdweb/src/react/web/ui/components/TokenIcon.tsx, packages/thirdweb/src/pay/buyWithFiat/isSwapRequiredPostOnramp.ts, packages/thirdweb/src/react/web/ui/ConnectWallet/screens/Buy/fiat/currencies.tsx, packages/thirdweb/src/exports/react-native.ts, packages/thirdweb/src/react/web/ui/ConnectWallet/screens/Buy/tx-history/TokenInfoRow.tsx, .changeset/little-coins-thank.md, packages/thirdweb/src/react/web/ui/ConnectWallet/screens/Buy/fiat/PostOnRampSwapFlow.tsx, packages/thirdweb/src/react/core/hooks/others/useTokenInfo.ts, packages/thirdweb/src/exports/pay.ts, packages/thirdweb/src/exports/react.ts, packages/thirdweb/src/react/web/ui/ConnectWallet/screens/Buy/Stepper.tsx, packages/thirdweb/src/react/core/hooks/pay/useBuyHistory.ts, packages/thirdweb/src/react/core/hooks/pay/useBuyWithFiatHistory.ts, packages/thirdweb/src/react/web/ui/ConnectWallet/screens/Buy/main/useBuyTxStates.ts, packages/thirdweb/src/react/core/hooks/pay/useBuyWithFiatStatus.ts, packages/thirdweb/src/react/web/ui/ConnectWallet/screens/SendFunds.tsx, packages/thirdweb/src/react/web/ui/ConnectWallet/screens/Buy/swap/Fees.tsx, packages/thirdweb/src/pay/utils/definitions.ts, packages/thirdweb/src/react/web/ui/ConnectWallet/icons/currencies/USDIcon.tsx, packages/thirdweb/src/pay/buyWithCrypto/actions/getQuote.ts, packages/thirdweb/src/react/web/ui/ConnectWallet/screens/Buy/PaymentSelection.tsx, packages/thirdweb/src/react/core/hooks/pay/usePostOnrampQuote.ts, packages/thirdweb/src/react/web/ui/ConnectWallet/screens/Buy/EstimatedTimeAndFees.tsx, packages/thirdweb/src/react/web/ui/ConnectWallet/ConnectButtonProps.ts, legacy_packages/react/src/wallet/ConnectWallet/screens/SwapTransactionsScreen.tsx, packages/thirdweb/src/react/web/ui/ConnectWallet/icons/currencies/GBPIcon.tsx, packages/thirdweb/src/react/web/ui/ConnectWallet/screens/Buy/swap/pendingSwapTx.ts, packages/thirdweb/src/react/core/hooks/pay/useBuyWithCryptoHistory.ts, packages/thirdweb/src/react/web/ui/ConnectWallet/screens/Buy/main/useEnabledPaymentMethods.ts, packages/thirdweb/src/react/web/ui/ConnectWallet/screens/Buy/PayWIthCreditCard.tsx, packages/thirdweb/src/react/web/ui/ConnectWallet/screens/Buy/tx-history/statusMeta.ts, packages/thirdweb/src/react/web/ui/ConnectWallet/screens/ViewFunds.tsx, packages/thirdweb/src/react/web/ui/ConnectWallet/screens/Buy/swap/PayWithCrypto.tsx, packages/thirdweb/src/pay/getBuyHistory.ts, packages/thirdweb/src/react/web/ui/ConnectWallet/screens/Buy/tx-history/BuyTxHistoryButton.tsx, packages/thirdweb/src/react/web/ui/ConnectWallet/screens/Buy/fiat/FiatTxDetailsTable.tsx, packages/thirdweb/src/react/web/ui/ConnectWallet/screens/Buy/fiat/FiatFlow.tsx, packages/thirdweb/src/react/web/ui/ConnectWallet/screens/Buy/swap/SwapFlow.tsx, packages/thirdweb/src/pay/buyWithFiat/getHistory.ts, packages/thirdweb/src/react/web/ui/ConnectWallet/screens/Buy/swap/useSwapSupportedChains.ts, packages/thirdweb/src/react/core/hooks/pay/useBuyWithFiatQuote.ts, packages/thirdweb/src/react/web/ui/ConnectWallet/screens/Buy/main/useUISelectionStates.ts, packages/thirdweb/src/react/web/ui/ConnectWallet/screens/Buy/tx-history/FiatDetailsScreen.tsx, packages/thirdweb/src/pay/buyWithFiat/getPostOnRampQuote.ts, packages/thirdweb/src/react/web/ui/ConnectWallet/screens/Buy/fiat/PostOnRampSwap.tsx, packages/thirdweb/src/react/web/ui/ConnectWallet/screens/Buy/tx-history/useBuyTransactionsToShow.ts, packages/thirdweb/src/react/core/hooks/pay/useBuyWithCryptoStatus.ts, packages/thirdweb/src/pay/buyWithCrypto/actions/getStatus.ts, packages/thirdweb/src/react/web/ui/ConnectWallet/screens/Buy/swap/SwapStatusScreen.tsx, packages/thirdweb/src/react/web/ui/ConnectWallet/screens/Buy/tx-history/BuyTxHistory.tsx, packages/thirdweb/src/react/web/ui/ConnectWallet/screens/TokenSelector.tsx, packages/thirdweb/src/react/core/hooks/pay/useBuyWithCryptoQuote.ts, packages/thirdweb/src/react/web/ui/ConnectWallet/Details.tsx, packages/thirdweb/src/react/web/ui/ConnectWallet/screens/Buy/fiat/FiatStatusScreen.tsx, packages/thirdweb/src/react/web/ui/ConnectWallet/screens/Buy/tx-history/SwapDetailsScreen.tsx, packages/thirdweb/src/react/web/hooks/useSendTransaction.tsx, packages/thirdweb/src/pay/buyWithFiat/getStatus.ts, packages/thirdweb/src/react/web/ui/PayEmbed.tsx, packages/thirdweb/src/pay/buyWithFiat/getQuote.ts, packages/thirdweb/src/react/web/ui/ConnectWallet/screens/Buy/fiat/FiatSteps.tsx, packages/thirdweb/src/react/web/ui/ConnectWallet/screens/Buy/swap/ConfirmationScreen.tsx, packages/thirdweb/src/react/web/ui/ConnectWallet/screens/Buy/BuyScreen.tsx

✨ Ask PR-Codex anything about this PR by commenting with /codex {your question}

@MananTank MananTank requested a review from IDubuque as a code owner April 17, 2024 19:33
@changeset-bot
Copy link

changeset-bot bot commented Apr 17, 2024

🦋 Changeset detected

Latest commit: 35adf7a

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 10 packages
Name Type
@thirdweb-dev/react Patch
thirdweb Minor
@thirdweb-dev/sdk Patch
@thirdweb-dev/cli Patch
@thirdweb-dev/react-core Patch
@thirdweb-dev/react-native Patch
@thirdweb-dev/unity-js-bridge Patch
@thirdweb-dev/wallets Patch
@thirdweb-dev/auth Patch
@thirdweb-dev/react-native-compat Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@MananTank MananTank marked this pull request as draft April 17, 2024 19:33
@codspeed-hq
Copy link

codspeed-hq bot commented Apr 17, 2024

CodSpeed Performance Report

Merging #2801 will not alter performance

Comparing mnn/buy-with-credit-card (35adf7a) with main (aad5f9f)

Summary

✅ 9 untouched benchmarks

@github-actions
Copy link
Contributor

github-actions bot commented Apr 17, 2024

size-limit report 📦

Path Size Loading time (3g) Running time (snapdragon) Total time
thirdweb (esm) 39.07 KB (+0.19% 🔺) 782 ms (+0.19% 🔺) 654 ms (+85.06% 🔺) 1.5 s
thirdweb (cjs) 87.91 KB (+0.17% 🔺) 1.8 s (+0.17% 🔺) 1.4 s (+10.01% 🔺) 3.2 s
thirdweb (minimal + tree-shaking) 4.71 KB (0%) 95 ms (0%) 37 ms (-1.85% 🔽) 131 ms
thirdweb/chains (tree-shaking) 400 B (0%) 10 ms (0%) 10 ms (-28.45% 🔽) 20 ms
thirdweb/react (minimal + tree-shaking) 16.38 KB (-0.05% 🔽) 328 ms (-0.05% 🔽) 89 ms (-45.02% 🔽) 417 ms

@codecov
Copy link

codecov bot commented Apr 18, 2024

Codecov Report

Attention: Patch coverage is 32.48369% with 3311 lines in your changes are missing coverage. Please review.

Project coverage is 64.54%. Comparing base (aad5f9f) to head (35adf7a).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2801      +/-   ##
==========================================
- Coverage   65.12%   64.54%   -0.58%     
==========================================
  Files         744      772      +28     
  Lines       52049    54159    +2110     
  Branches     3091     3092       +1     
==========================================
+ Hits        33896    34957    +1061     
- Misses      17488    18536    +1048     
- Partials      665      666       +1     
Flag Coverage Δ *Carryforward flag
legacy_packages 65.65% <ø> (-0.01%) ⬇️ Carriedforward from 0e4391e
packages 64.26% <32.48%> (-0.72%) ⬇️

*This pull request uses carry forward flags. Click here to find out more.

Files Coverage Δ
...kages/thirdweb/src/pay/buyWithCrypto/getHistory.ts 71.42% <100.00%> (ø)
...b/src/react/web/ui/ConnectWallet/ConnectButton.tsx 58.19% <100.00%> (ø)
...eb/src/react/web/ui/ConnectWallet/defaultTokens.ts 100.00% <100.00%> (ø)
...thirdweb/src/react/web/ui/components/ChainIcon.tsx 48.52% <ø> (-0.75%) ⬇️
...hirdweb/src/react/web/ui/design-system/elements.ts 100.00% <100.00%> (ø)
...ckages/thirdweb/src/pay/buyWithCrypto/getStatus.ts 81.37% <97.72%> (ø)
.../react/web/ui/ConnectWallet/screens/nativeToken.ts 75.00% <0.00%> (ø)
...ui/ConnectWallet/screens/Buy/swap/pendingSwapTx.ts 83.33% <78.57%> (+47.84%) ⬆️
...ackages/thirdweb/src/pay/buyWithCrypto/getQuote.ts 68.99% <80.00%> (ø)
...rc/react/core/hooks/contract/useSendTransaction.ts 33.57% <20.00%> (+0.71%) ⬆️
... and 53 more

... and 8 files with indirect coverage changes

@MananTank
Copy link
Member Author

/release-pr

@MananTank
Copy link
Member Author

/release-pr

@MananTank
Copy link
Member Author

/release-pr

@MananTank
Copy link
Member Author

/release-pr

@MananTank
Copy link
Member Author

/release-pr

@MananTank
Copy link
Member Author

/release-pr

@MananTank
Copy link
Member Author

/release-pr

// Must pass 0 otherwise it will throw on some chains
const gasCost = await estimateGasCost({
transaction: { ...tx, value: 0n },
transaction: tx, // NEED A PROPER FIX HERE !!!
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what was the issue here?

Copy link
Member Author

@MananTank MananTank May 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The value: 0n override was added because estimateGasCost was throwing errors on Optimism, Base, Arbitrum without it.

@MananTank
Copy link
Member Author

/release-pr

Copy link
Member

@jnsdls jnsdls left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

review part(1/n)

@MananTank
Copy link
Member Author

PR is not ready for review yet - I'm still reviewing

client={data.tx.client}
localeId={payModal?.locale || "en_US"}
supportedTokens={payModal?.supportedTokens || defaultTokens}
supportedTokens={payModal?.supportedTokens}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we don't default to defaultTokens here anymore?

@joaquim-verges joaquim-verges marked this pull request as ready for review May 10, 2024 03:11
@joaquim-verges joaquim-verges enabled auto-merge May 10, 2024 03:13
@joaquim-verges joaquim-verges merged commit 0aae34b into main May 10, 2024
@joaquim-verges joaquim-verges deleted the mnn/buy-with-credit-card branch May 10, 2024 03:57
@jnsdls jnsdls mentioned this pull request May 10, 2024
github-merge-queue bot pushed a commit that referenced this pull request May 10, 2024
joaquim-verges pushed a commit that referenced this pull request May 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants